Skip to content

Replace std::error::Error with core::error::Error #116

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

divyaranjan1905
Copy link

This PR simply replaces instances of std::error::Error with core::error::Error. It fixes #81, but as mentioned in the issue, it should probably be merged after #115.

Regards,

Divya.

@divyaranjan1905 divyaranjan1905 requested a review from a team as a code owner April 2, 2025 05:20
@joncinque joncinque added the breaking PR contains breaking changes label Apr 2, 2025
hash/src/lib.rs Outdated
@@ -95,7 +95,7 @@ pub enum ParseHashError {
}

#[cfg(feature = "std")]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i guess it can be dropped

@@ -361,7 +361,7 @@ impl FromPrimitive for ParsePubkeyError {
}

#[cfg(feature = "std")]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the same here

@@ -103,7 +103,7 @@ impl FromPrimitive for PubkeyError {
}

#[cfg(feature = "std")]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here

@@ -442,7 +442,7 @@ pub enum LamportsError {
}

#[cfg(feature = "std")]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and this one also

@@ -243,7 +243,7 @@ pub enum InstructionError {
}

#[cfg(feature = "std")]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one more place

@@ -243,7 +243,7 @@ pub enum InstructionError {
}

#[cfg(feature = "std")]
impl std::error::Error for InstructionError {}
impl core::error::Error for InstructionError {}

#[cfg(feature = "std")]

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is also can be omitted, because this is a core::fmt::Dispalay trait

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I originally considered dropping these attributes, but I wasn't sure. Thank you for confirming.

divyaranjan1905 and others added 4 commits April 2, 2025 18:21
* CI: Add powerset clippy check

As pointed out in anza-xyz#108, we don't explicitly check for all combinations
of features in the sdk, and more specifically, we miss warnings.

Add a CI step to run `cargo clippy -- --deny=warnings` on the powerset
of features in the sdk. Currently, this means 888 different crate
builds, but most of them are quite short.

Let's see how long it takes to run in CI to see if it's worth adding.

Also, fix some little issues that came up while running this.

* Oops, also install clippy

* Also fixup the name of the cache key

* Remove the bincode requirement for `SlotHashesSysvar`

* Fixup signature error
#### Problem

Rust v1.85.1 is now available, but the sdk repo is still on 1.84.1

#### Summary of changes

Bump the version declared in rust-toolchain.toml
* CI: Remove nightly check, add more to hack check

#### Problem

We've been adding a lot of new CI steps to the SDK, but some of them
might be redundant, causing us to use more CI resources than necessary.

For example, we run `./cargo nightly check` and `./cargo nightly hack
check`. Are both of those really necessary?

#### Summary of changes

Remove the nightly check CI job and script, and add some of those
features and flags to the hack check.

NOTE: I'm not 100% sure about this, so let me know what you think!

* Rename hack step to check for simplicity
@divyaranjan1905
Copy link
Author

divyaranjan1905 commented Apr 2, 2025

Rebased the last few commits and removed the feature attributes for std.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking PR contains breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MSRV: Use core::error::Error instead of std::error::Error
3 participants